Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[409] end of cycle implement a single deadline for all candidates #9503

Merged

Conversation

elceebee
Copy link
Contributor

@elceebee elceebee commented Jun 25, 2024

Context

This card to remove the 'apply_2' deadline was created in July last year during the continuous application work. Now that we have started doing the end of cycle work again this year, it is apparent that this apply_1 and apply_2 logic creates confusion and should be removed before we move forward.

Changes proposed in this pull request

Broadly, the PR just:

  • adds an apply_deadline to the cycle definitions
  • replaces methods in the CycleTimeable service to remove the distinction between the two deadlines.

For backward compatibility, I have retained the apply_1 and apply_2 deadlines on those cycles where it was relevant.

What IS NOT in this PR (tickets to be written for the following):

  • Completely remove the apply_again concept (apply_again is defined throughout the code base as apply_2 applications with submitted statuses. There are no apply_2 applications now, so this is irrelevant, but too big for this pr
  • Remove the phase column from the application. This is 'apply_1' for all applications so just extra noise that we should remove. But too big for this ticket.
  • Change the way we store / maintain cycle deadlines. We need to decide how far back we want to keep this data and if we are keeping ad infinitum, we should start storing it in the database rather just a constant in the codebase.

Guidance to review

I've got the test suite passing and I've done lots of manual testing locally. Would appreciate other people to do lots of clicking and see if I've broken anything.

It's a big PR, so best to review commit by commit.

Link to Trello card

https://trello.com/c/FPph5syu

Things to check

  • If the code removes any existing feature flags, a data migration has also been added to delete the entry from the database
  • This code does not rely on migrations in the same Pull Request
  • If this code includes a migration adding or changing columns, it also backfills existing records for consistency
  • If this code adds a column to the DB, decide whether it needs to be in analytics yml file or analytics blocklist
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Required environment variables have been updated added to the Azure KeyVault

@elceebee elceebee force-pushed the 409-end-of-cycle-implement-a-single-deadline-for-all-candidates branch from 95e347e to e146613 Compare June 26, 2024 08:21
@elceebee elceebee force-pushed the 409-end-of-cycle-implement-a-single-deadline-for-all-candidates branch from e146613 to b937e81 Compare June 26, 2024 08:22
@elceebee elceebee force-pushed the 409-end-of-cycle-implement-a-single-deadline-for-all-candidates branch from b937e81 to a66fc38 Compare June 26, 2024 08:48
@elceebee elceebee force-pushed the 409-end-of-cycle-implement-a-single-deadline-for-all-candidates branch from a66fc38 to 530ed9f Compare June 26, 2024 10:37
@elceebee
Copy link
Contributor Author

@elceebee elceebee self-assigned this Jun 26, 2024
@elceebee elceebee added the deploy_v2 Deploy the review app to AKS label Jun 26, 2024
@elceebee elceebee marked this pull request as ready for review June 26, 2024 13:13
@elceebee elceebee changed the title DRAFT 409 end of cycle implement a single deadline for all candidates [409] end of cycle implement a single deadline for all candidates Jun 26, 2024
@github-actions github-actions bot temporarily deployed to review_aks-9503 June 26, 2024 13:16 Destroyed
@github-actions github-actions bot temporarily deployed to review_aks-9503 June 26, 2024 13:42 Destroyed
@elceebee elceebee force-pushed the 409-end-of-cycle-implement-a-single-deadline-for-all-candidates branch from ee5b4d6 to f1d2c5e Compare June 27, 2024 08:32
@github-actions github-actions bot temporarily deployed to review_aks-9503 June 27, 2024 08:37 Destroyed
@github-actions github-actions bot temporarily deployed to review_aks-9503 June 27, 2024 08:49 Destroyed
@elceebee elceebee force-pushed the 409-end-of-cycle-implement-a-single-deadline-for-all-candidates branch from bac7e26 to 83c3615 Compare June 27, 2024 09:02
@github-actions github-actions bot temporarily deployed to review_aks-9503 June 27, 2024 12:45 Destroyed
@elceebee elceebee merged commit a2b1a27 into main Jun 27, 2024
24 of 26 checks passed
@elceebee elceebee deleted the 409-end-of-cycle-implement-a-single-deadline-for-all-candidates branch June 27, 2024 14:05
Copy link

sentry-io bot commented Jul 1, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: relation "site_settings" does not exist (ActiveRecord::StatementInvalid) Sidekiq/StartOfCycleNotificationWorker View Issue
  • ‼️ ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: relation "site_settings" does not exist (ActiveRecord::StatementInvalid) Sidekiq/ChaseReferences View Issue
  • ‼️ ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: relation "site_settings" does not exist (ActiveRecord::StatementInvalid) app/models/site_setting.rb in cycle_schedule View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy_v2 Deploy the review app to AKS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants